Skip to content

Conversation

@dkarlovi
Copy link
Contributor

Closes #767.

@DannyvdSluijs
Copy link
Collaborator

@dkarlovi I think this PR could be accepted. If you could just do a rebase and add a changelog entry in CHANGELOG.md?

Maybe good to know we are seeing multiple issues being reported with associative arrays, which is a concept that doesn't live in JSON. With those two facts we're seeing if we should deprecate the associative array support in the next mayor version 7.0.0

@dkarlovi
Copy link
Contributor Author

I'm all for deprecating the support as mentioned before. 👍

Comment on lines 61 to 67
if (is_object($schema) && property_exists($schema, 'id')) {
$schemaURI = $schema->id;
} elseif (is_array($schema) && array_key_exists('id', $schema)) {
$schemaURI = $schema['id'];
} else {
$schemaURI = SchemaStorage::INTERNAL_PROVIDED_SCHEMA_URI;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I've learned something more about the codebase today. These changes should be done respecting the CHECK_MODE_TYPE_CAST. Doing either a loose type check or a strict type check. I believe the following would do so (but requires some testing as well)

Suggested change
if (is_object($schema) && property_exists($schema, 'id')) {
$schemaURI = $schema->id;
} elseif (is_array($schema) && array_key_exists('id', $schema)) {
$schemaURI = $schema['id'];
} else {
$schemaURI = SchemaStorage::INTERNAL_PROVIDED_SCHEMA_URI;
}
$schemaURI = SchemaStorage::INTERNAL_PROVIDED_SCHEMA_URI;
if ($this->factory->getTypeCheck()::propertyExists($schema, 'id')) {
$schemaURI = $this->factory->getTypeCheck()::propertyGet($schema, 'id');
}

@DannyvdSluijs
Copy link
Collaborator

This PR has been replaced by #794

DannyvdSluijs added a commit that referenced this pull request Feb 26, 2025
DannyvdSluijs added a commit that referenced this pull request Feb 26, 2025
Replaces #769 due to lack of permissions to update PR
Closes #767.

---------

Co-authored-by: Dalibor Karlović <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validator::validate() doesn't correctly apply schema ID when the schema is an assoc array (works when an object)

2 participants